Skip to content

#1231 Add API endpoints for messages uploading#1261

Merged
tomholub merged 11 commits intomasterfrom
feature/issue-1615-password-api
Dec 30, 2021
Merged

#1231 Add API endpoints for messages uploading#1261
tomholub merged 11 commits intomasterfrom
feature/issue-1615-password-api

Conversation

@sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented Dec 30, 2021

This PR adds ability to upload messages to FES web portal.

close #1231


Tests (delete all except exactly one):

  • Difficult to test (explain why) - it depends on API responses, probably we'll be able to test it later with UI Tests

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@sosnovsky sosnovsky requested a review from tomholub December 30, 2021 14:14
Comment on lines +166 to +173
private func getIdToken(email: String) async throws -> String {
let googleService = GoogleUserService(
currentUserEmail: email,
appDelegateGoogleSessionContainer: nil
)

return try await googleService.getCachedOrRefreshedIdToken()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather the idToken be passed as argument into the methods (instead of email). But probably ok for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently getIdToken is used only in performRequest, so we don't need to manually fetch token on each request. Looks like a simpler solution

@sosnovsky sosnovsky requested a review from tomholub December 30, 2021 21:43

let response: MessageUploadResponse = try await performRequest(
email: sender,
email: details.from,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a subtle thing: from and current account may not be the same thing in the future - once we add support for different sendAs aliases, in which case you may send an email from alias@example.com while on account account@example.com. So this would really need an account and from separately, or in the future maybe idToken and from separately. For now though, it's ok. I'll file an issue.


func upload(message: Data, sender: String, to: [String], cc: [String], bcc: [String]) async throws -> String {
func getReplyToken(for email: String) async throws -> String {
return ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better mock value would be mock-reply-token

}

func upload(message: Data, details: MessageUploadDetails) async throws -> String {
return ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better mock-web-portal-message-url

@tomholub tomholub enabled auto-merge (squash) December 30, 2021 21:59
@tomholub tomholub merged commit e9ad15f into master Dec 30, 2021
@tomholub tomholub deleted the feature/issue-1615-password-api branch December 30, 2021 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API class for calling FES endpoints to upload message to web portal

2 participants